fix graph building logic and complete /subgraph endpoint #60
Conversation
| * @param spanIdToSpans Lookup map from span ID to spans | ||
| * @param parentSpanIdToChildSpans Map from parent span ID to list of child spans | ||
| */ | ||
| private void dfsFilter( |
There was a problem hiding this comment.
I think this could probably do less work. It looks like it's traversing the full child graph from each filtered node for each filtered node, resulting in some duplicated work.
If you composed the algorithm a little differently, you could do it with oneish traversal total using an approach like this:
filtered-nodes = [...]
edges = []
for n in filtered-nodes:
transitive-matching-children = collect-transitive-matching-children(n, filtered-nodes)
for c in transitive-matching-children:
edges << create-edge(n, c)
def collect-transitive-matching-children(n, filtered-nodes):
results = []-no
work=[n]
while work not empty:
current = work.pop
for c in get-children(current):
if c not in filtered-nodes:
work.push(c)
else:
results.push(c)
You could add a visited check too in case there could be cycles, but I think that'll be more of a possibility in the derived graph than the original.
| return options.entrySet().stream() | ||
| .filter(entry -> entry.getValue() != null && !entry.getValue().isEmpty()) |
There was a problem hiding this comment.
you could move doing this filter into the constructor, instead of doing it for each span
| spanIdToSpans.put(span.getId(), span); | ||
|
|
||
| String parentId = span.getParentId(); | ||
| if (parentId != null) { |
There was a problem hiding this comment.
I think -1 is also a value that means missing. We might want to clean that up, but you might want to check for it.
| } | ||
| } | ||
|
|
||
| long start = System.currentTimeMillis(); |
There was a problem hiding this comment.
you could use io.micrometer.core.instrument.Timer for this, then you could wire it up as a metric in addition to reporting it in the response.
Summary
This PR updates
GraphBuilderandGraphConfigto align with the new span <> metadata configuration and moves certain post-processing logic into Astra for correctness.GraphConfigGraphBuilderPost-Processing within Astra
Added a post-processing DFS step during dependency generation for specific span filters. This logic was originally planned to run outside Astra, but that approach lost the span hierarchy required to properly drop intermediate spans and reconnect relevant nodes.
Requirements
Testing